Skip to content

Use Filterset Factory in Project and Sample Views#1868

Open
nozomione wants to merge 8 commits intodevfrom
nozomone/1849-refactor-project-sample-endpoints
Open

Use Filterset Factory in Project and Sample Views#1868
nozomione wants to merge 8 commits intodevfrom
nozomone/1849-refactor-project-sample-endpoints

Conversation

@nozomione
Copy link
Member

@nozomione nozomione commented Mar 9, 2026

Issue Number

Related Issue #1849

Purpose/Implementation Notes

Changes include:

  • Added the bootstrapped FilterSet factory, filter module to the project root
  • Incorporated the filter module to auto-generate the filtersets with default lookup expressions for project and sample endpoints
  • Added the following field types to views:
    • All boolean fields included in the serilzier
    • All datetime fields included in the serializer
    • All integer fields included in the serializer

Types of changes

What types of changes does your code introduce?

  • Bugfix (non-breaking change which fixes an issue)
  • Refactor (addresses code organization and design mentioned in corresponding issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Functional tests

  • Test and verify the functionality on localhost
    e.g. ) http://localhost:8000/v1/projects?diagnoses=Glioblastoma,Ependymoma

Checklist

  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • Any dependent changes have been merged and published in downstream modules

Screenshots

N/A

@nozomione nozomione self-assigned this Mar 9, 2026
@nozomione nozomione requested a review from davidsmejia as a code owner March 9, 2026 14:19
@nozomione nozomione added the API label Mar 9, 2026
@nozomione nozomione marked this pull request as ready for review March 12, 2026 15:47
Sample,
include_fields=[
"scpca_id",
"project__scpca_id",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"project__scpca_id",

# timestamps
"created_at",
"updated_at",
],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
],
],
extra_filters = {
"project__scpca_id": ["exact"],
},

@davidsmejia
Copy link
Contributor

Can you add a test to ensure that this case is handled

Comment on lines +73 to +77
# Add other custom lookup fields included in include_fields
if include_fields:
for field in include_fields:
if field not in meta_fields and field not in declared_filters:
meta_fields[field] = ["exact", "icontains"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Add other custom lookup fields included in include_fields
if include_fields:
for field in include_fields:
if field not in meta_fields and field not in declared_filters:
meta_fields[field] = ["exact", "icontains"]

…s parameter for additional fields that should be filterable via the public API, and adjust views' arguments accordingly
@nozomione
Copy link
Member Author

I've:

  • Updated filter.build_auto_filterset as follows:
    • Renamed include_fields to auto_fields to indicate that these are model fields for clarity
    • Added a new parameter, extra_fields, for additional fields that need to be included in the public API
      • Append them to meta_fields if provided
  • Added a corresponding test, test_filter, to verify the implementation (e.g., filter inclusions, default lookups support)

This PR is ready for another look. Thank you, David!

@nozomione nozomione requested a review from davidsmejia March 16, 2026 20:50
…ger checked as it uses the same TextField as 'scpca_id')
Copy link
Contributor

@davidsmejia davidsmejia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants